From 0b680daf28fffa1ad84cdb93ff2acf02ff58a972 Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Fri, 1 May 2020 06:32:18 -0600 Subject: [PATCH] add recoverymode and CRC checking to FIT reader. (#549) * add recoverymode and CRC checking to FIT reader. If present, the header CRC is checked. The file CRC and length is checked. A recoverymode option is added. In the default mode we will fatal with: a bad CRC, a bad endian field, an attempt to read when the data section doesn't have sufficient data, an unexepected EOF. In recovery mode when we encounter one of these errors we will abort read processing and continue. This allows a more immediate cleaner exit from the reader while still allowing any writer to use data that was recovered previous to the read abort. * add further explanation of recoverymode for document. * make sure garmin fit messages are defined before being used. --- garmin_fit.cc | 221 ++++++++++++------ garmin_fit.h | 21 +- reference/format3.txt | 2 + reference/help.txt | 1 + testo.d/garmin_fit.test | 2 +- .../options/garmin_fit-recoverymode.xml | 13 ++ 6 files changed, 186 insertions(+), 74 deletions(-) create mode 100644 xmldoc/formats/options/garmin_fit-recoverymode.xml diff --git a/garmin_fit.cc b/garmin_fit.cc index 32ccb3f79..d880056da 100644 --- a/garmin_fit.cc +++ b/garmin_fit.cc @@ -22,22 +22,27 @@ */ -#include // for uint8_t, uint16_t, uint32_t, int32_t, int8_t, uint64_t -#include // for EOF, SEEK_SET, snprintf -#include // for deque, _Deque_iterator, operator!= -#include // for allocator_traits<>::value_type -#include // for pair -#include // for vector - -#include // for QByteArray -#include // for QDateTime -#include // for QString -#include // for CaseInsensitive +#include // for uint8_t, uint16_t, uint32_t, int32_t, int8_t, uint64_t +#include // for EOF, SEEK_SET, snprintf +#include // for deque, _Deque_iterator, operator!= +#include // for allocator_traits<>::value_type +#include // for operator+, to_string, char_traits +#include // for pair +#include // for vector + +#include // for QByteArray +#include // for QDateTime +#include // for QFileInfo +#include // for QLatin1Char +#include // for QString +#include // for CaseInsensitive +#include // for qint64 #include "defs.h" #include "garmin_fit.h" -#include "gbfile.h" // for gbfputc, gbfputuint16, gbfputuint32, gbfgetc, gbfread, gbfseek, gbfclose, gbfgetuint16, gbfopen_le, gbfputint32, gbfflush, gbfgetuint32, gbfputs, gbftell, gbfwrite, gbfile, gbsize_t -#include "jeeps/gpsmath.h" // for GPS_Math_Semi_To_Deg, GPS_Math_Gtime_To_Utime, GPS_Math_Deg_To_Semi, GPS_Math_Utime_To_Gtime +#include "gbfile.h" // for gbfputc, gbfputuint16, gbfputuint32, gbfgetc, gbfread, gbfseek, gbfclose, gbfgetuint16, gbfopen_le, gbfputint32, gbfflush, gbfgetuint32, gbfputs, gbftell, gbfwrite, gbfile, gbsize_t +#include "jeeps/gpsmath.h" // for GPS_Math_Semi_To_Deg, GPS_Math_Gtime_To_Utime, GPS_Math_Deg_To_Semi, GPS_Math_Utime_To_Gtime +#include "src/core/logging.h" // for Warning, Fatal #define MYNAME "fit" @@ -67,10 +72,7 @@ GarminFitFormat::rd_init(const QString& fname) void GarminFitFormat::rd_deinit() { - for (auto& local_id : fit_data.message_def) { - // QList::clear() will not deallocate - local_id.fields = QList(); - } + fit_data = fit_data_t(); gbfclose(fin); } @@ -127,32 +129,57 @@ GarminFitFormat::fit_parse_header() debug_print(1,"%s: fit_data.len=%d\n", MYNAME, fit_data.len); } - if (len > 12) { - // Unused according to Ingo Arndt - gbfgetuint16(fin); + // Header CRC may be omitted entirely + if (len >= kReadHeaderCrcLen) { + uint16_t hdr_crc = gbfgetuint16(fin); + // Header CRC may be set to 0, or contain the CRC over previous bytes. + if (hdr_crc != 0) { + // Check the header CRC + uint16_t crc = 0; + gbfseek(fin, 0, SEEK_SET); + for (unsigned int i = 0; i < kReadHeaderCrcLen; ++i) { + int data = gbfgetc(fin); + if (data == EOF) { + fatal(MYNAME ": File %s truncated\n", fin->name); + } + crc = fit_crc16(data, crc); + } + if (crc != 0) { + Warning().nospace() << MYNAME ": Header CRC mismatch in file " << fin->name << "."; + if (!opt_recoverymode) { + Fatal().nospace() << MYNAME ": File " << fin->name << " is corrupt. Use recoverymode option at your risk."; + } + } else if (global_opts.debug_level >= 1) { + debug_print(1, MYNAME ": Header CRC verified.\n"); + } + } } + QFileInfo fi(fin->name); + qint64 size = fi.size(); + if ((len + fit_data.len + 2) != size) { + Warning() << MYNAME ": File size" << size << "is not expected given header len" << len << ", data length" << fit_data.len << "and a 2 byte file CRC."; + } else if (global_opts.debug_level >= 1) { + debug_print(1, MYNAME ": File size matches expectations from information in the header.\n"); + } + + gbfseek(fin, len, SEEK_SET); + fit_data.global_utc_offset = 0; } uint8_t GarminFitFormat::fit_getuint8() { - if (fit_data.len == 0) { - // fail gracefully for GARMIN Edge 800 with newest firmware, seems to write a wrong record length - // for the last record. - //fatal(MYNAME ": record truncated: fit_data.len=0\n"); - if (global_opts.debug_level >= 1) { - warning("%s: record truncated: fit_data.len=0\n", MYNAME); - } - return 0; + if (fit_data.len < 1) { + throw ReaderException("record truncated: expecting char[1], but only got " + std::to_string(fit_data.len) + "."); } int val = gbfgetc(fin); if (val == EOF) { - fatal(MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len); + throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + "."); } - fit_data.len--; - return (uint8_t)val; + --fit_data.len; + return static_cast(val); } uint16_t @@ -161,10 +188,12 @@ GarminFitFormat::fit_getuint16() char buf[2]; if (fit_data.len < 2) { - fatal(MYNAME ": record truncated: expecting char[2], but only got %d\n",fit_data.len); + throw ReaderException("record truncated: expecting char[2], but only got " + std::to_string(fit_data.len) + "."); + } + gbsize_t count = gbfread(buf, 2, 1, fin); + if (count != 1) { + throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + "."); } - is_fatal(gbfread(buf, 2, 1, fin) != 1, - MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len); fit_data.len -= 2; if (fit_data.endian) { return be_read16(buf); @@ -179,10 +208,12 @@ GarminFitFormat::fit_getuint32() char buf[4]; if (fit_data.len < 4) { - fatal(MYNAME ": record truncated: expecting char[4], but only got %d\n",fit_data.len); + throw ReaderException("record truncated: expecting char[4], but only got " + std::to_string(fit_data.len) + "."); + } + gbsize_t count = gbfread(buf, 4, 1, fin); + if (count != 1) { + throw ReaderException("unexpected end of file with fit_data.len=" + std::to_string(fit_data.len) + "."); } - is_fatal(gbfread(buf, 4, 1, fin) != 1, - MYNAME ": unexpected end of file with fit_data.len=%d\n",fit_data.len); fit_data.len -= 4; if (fit_data.endian) { return be_read32(buf); @@ -195,23 +226,21 @@ void GarminFitFormat::fit_parse_definition_message(uint8_t header) { int local_id = header & 0x0f; - fit_message_def* def = &fit_data.message_def[local_id]; - - def->fields = QList(); + fit_message_def def; // first byte is reserved. It's usually 0 and we don't know what it is, // but we've seen some files that are 0x40. So we just read it and toss it. (void) fit_getuint8(); // second byte is endianness - def->endian = fit_getuint8(); - if (def->endian > 1) { - warning(MYNAME ": Unusual endian field (interpreting as big endian): %d\n",def->endian); + def.endian = fit_getuint8(); + if (def.endian > 1) { + throw ReaderException(QString("Bad endian field 0x%1 at file position 0x%2.").arg(def.endian, 0, 16).arg(gbftell(fin) - 1, 0, 16).toStdString()); } - fit_data.endian = def->endian; + fit_data.endian = def.endian; // next two bytes are the global message number - def->global_id = fit_getuint16(); + def.global_id = fit_getuint16(); // byte 5 has the number of records in the remainder of the definition message int num_fields = fit_getuint8(); @@ -229,7 +258,7 @@ GarminFitFormat::fit_parse_definition_message(uint8_t header) debug_print(8,"%s: record %d ID: %d SIZE: %d TYPE: %d fit_data.len=%d\n", MYNAME, i, field.id, field.size, field.type, fit_data.len); } - def->fields.append(field); + def.fields.append(field); } // If we have developer fields (since version 2.0) they must be read too @@ -259,26 +288,26 @@ GarminFitFormat::fit_parse_definition_message(uint8_t header) if (global_opts.debug_level >= 8) { debug_print(8,"%s: definition message contains %d developer records\n",MYNAME, numOfDevFields); } - if (numOfDevFields == 0) { - return; - } - - int numOfFields = num_fields + numOfDevFields; - for (int i = num_fields; i < numOfFields; ++i) { - int id = fit_getuint8(); - int size = fit_getuint8(); - int type = fit_getuint8(); - fit_field_t field = {id, size, type}; - if (global_opts.debug_level >= 8) { - debug_print(8,"%s: developer record %d ID: %d SIZE: %d TYPE: %d fit_data.len=%d\n", - MYNAME, i - num_fields, field.id, field.size, field.type, fit_data.len); + if (numOfDevFields > 0) { + int numOfFields = num_fields + numOfDevFields; + for (int i = num_fields; i < numOfFields; ++i) { + int id = fit_getuint8(); + int size = fit_getuint8(); + int type = fit_getuint8(); + fit_field_t field = {id, size, type}; + if (global_opts.debug_level >= 8) { + debug_print(8,"%s: developer record %d ID: %d SIZE: %d TYPE: %d fit_data.len=%d\n", + MYNAME, i - num_fields, field.id, field.size, field.type, fit_data.len); + } + // Because we parse developer fields like normal fields and we do not want + // that the field id interfere which valid id's from the normal fields + field.id = kFieldInvalid; + def.fields.append(field); } - // Because we parse developer fields like normal fields and we do not want - // that the field id interfere which valid id's from the normal fields - field.id = kFieldInvalid; - def->fields.append(field); } } + + fit_data.message_def.insert(local_id, def); } uint32_t @@ -645,14 +674,26 @@ void GarminFitFormat::fit_parse_data_message(uint8_t header) { int local_id = header & 0x0f; - fit_parse_data(fit_data.message_def[local_id], 0); + if (fit_data.message_def.contains(local_id)) { + fit_parse_data(fit_data.message_def.value(local_id), 0); + } else { + throw ReaderException( + QString("Message %1 hasn't been defined before being used at file position 0x%2."). + arg(local_id).arg(gbftell(fin) - 1, 0, 16).toStdString()); + } } void GarminFitFormat::fit_parse_compressed_message(uint8_t header) { int local_id = (header >> 5) & 3; - fit_parse_data(fit_data.message_def[local_id], header & 0x1f); + if (fit_data.message_def.contains(local_id)) { + fit_parse_data(fit_data.message_def.value(local_id), header & 0x1f); + } else { + throw ReaderException( + QString("Compressed message %1 hasn't been defined before being used at file position 0x%2."). + arg(local_id).arg(gbftell(fin) - 1, 0, 16).toStdString()); + } } /******************************************************************************* @@ -661,6 +702,7 @@ GarminFitFormat::fit_parse_compressed_message(uint8_t header) void GarminFitFormat::fit_parse_record() { + gbsize_t position = gbftell(fin); uint8_t header = fit_getuint8(); // high bit 7 set -> compressed message (0 for normal) // second bit 6 set -> 0 for data message, 1 for definition message @@ -672,25 +714,53 @@ GarminFitFormat::fit_parse_record() // bits 3..0 -> local message type if (header & 0x80) { if (global_opts.debug_level >= 6) { - debug_print(6,"%s: got compressed message at fit_data.len=%d", MYNAME, fit_data.len); + debug_print(6,"%s: got compressed message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len); debug_print(0," ...local message type 0x%X\n", header&0x0f); } fit_parse_compressed_message(header); } else if (header & 0x40) { if (global_opts.debug_level >= 6) { - debug_print(6,"%s: got definition message at fit_data.len=%d", MYNAME, fit_data.len); + debug_print(6,"%s: got definition message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len); debug_print(0," ...local message type 0x%X\n", header&0x0f); } fit_parse_definition_message(header); } else { if (global_opts.debug_level >= 6) { - debug_print(6,"%s: got data message at fit_data.len=%d", MYNAME, fit_data.len); + debug_print(6,"%s: got data message at file position 0x%x, fit_data.len=%d", MYNAME, position, fit_data.len); debug_print(0," ...local message type 0x%X\n", header&0x0f); } fit_parse_data_message(header); } } +void +GarminFitFormat::fit_check_file_crc() const +{ + // Check file CRC + + gbsize_t position = gbftell(fin); + + uint16_t crc = 0; + gbfseek(fin, 0, SEEK_SET); + while (true) { + int data = gbfgetc(fin); + if (data == EOF) { + break; + } + crc = fit_crc16(data, crc); + } + if (crc != 0) { + Warning().nospace() << MYNAME ": File CRC mismatch in file " << fin->name << "."; + if (!opt_recoverymode) { + Fatal().nospace() << MYNAME ": File " << fin->name << " is corrupt. Use recoverymode option at your risk."; + } + } else if (global_opts.debug_level >= 1) { + debug_print(1, MYNAME ": File CRC verified.\n"); + } + + gbfseek(fin, position, SEEK_SET); +} + /******************************************************************************* * fit_read- global entry point * - parse the header @@ -699,6 +769,8 @@ GarminFitFormat::fit_parse_record() void GarminFitFormat::read() { + fit_check_file_crc(); + fit_parse_header(); fit_data.track = new route_head; @@ -706,8 +778,17 @@ GarminFitFormat::read() if (global_opts.debug_level >= 1) { debug_print(1,"%s: starting to read data with fit_data.len=%d\n", MYNAME, fit_data.len); } - while (fit_data.len) { - fit_parse_record(); + try { + while (fit_data.len) { + fit_parse_record(); + } + } catch (ReaderException& e) { + if (opt_recoverymode) { + warning(MYNAME ": %s\n",e.what()); + warning(MYNAME ": Aborting read and continuning processing.\n"); + } else { + fatal(MYNAME ": %s Use recoverymode option at your risk.\n",e.what()); + } } } diff --git a/garmin_fit.h b/garmin_fit.h index e5f697c42..4cbd8d8cb 100644 --- a/garmin_fit.h +++ b/garmin_fit.h @@ -26,9 +26,11 @@ #include // for uint8_t, uint16_t, uint32_t #include // for deque +#include // for runtime_error #include // for pair #include // for vector +#include // for QHash #include // for QList #include // for QString #include // for QVector @@ -82,12 +84,12 @@ private: /* Types */ struct fit_field_t { - /* MSVC 2015 generates C2664 errors without some help. */ + /* MSVC 2015 generates C2664 errors without some help. */ #if defined(_MSC_VER) && (_MSC_VER < 1910) /* MSVC 2015 or earlier */ fit_field_t() = default; fit_field_t(int i, int s, int t) : id(i), size(s), type(t) {} #endif - int id{}; + int id {}; int size{}; int type{}; }; @@ -104,7 +106,7 @@ private: route_head* track{nullptr}; uint32_t last_timestamp{}; uint32_t global_utc_offset{}; - fit_message_def message_def[16]; + QHash message_def; }; struct FitCourseRecordPoint { @@ -126,6 +128,11 @@ private: unsigned int course_point_type; }; + class ReaderException : public std::runtime_error + { + using std::runtime_error::runtime_error; + }; + /* Constants */ // constants for global IDs @@ -218,6 +225,7 @@ private: static constexpr int kWriteHeaderLen = 12; static constexpr int kWriteHeaderCrcLen = 14; + static constexpr int kReadHeaderCrcLen = 14; static constexpr double kSynthSpeed = 10.0 * 1000 / 3600; /* speed in m/s */ @@ -233,6 +241,7 @@ private: void fit_parse_data_message(uint8_t header); void fit_parse_compressed_message(uint8_t header); void fit_parse_record(); + void fit_check_file_crc() const; void fit_write_message_def(uint8_t local_id, uint16_t global_id, const std::vector& fields) const; static uint16_t fit_crc16(uint8_t data, uint16_t crc); void fit_write_timestamp(const gpsbabel::DateTime& t) const; @@ -255,6 +264,7 @@ private: /* Data Members */ char* opt_allpoints = nullptr; + char* opt_recoverymode = nullptr; int lap_ct = 0; bool new_trkseg = false; bool write_header_msgs = false; @@ -265,6 +275,11 @@ private: "Read all points even if latitude or longitude is missing", nullptr, ARGTYPE_BOOL, ARG_NOMINMAX, nullptr }, + { + "recoverymode", &opt_recoverymode, + "Attempt to recovery data from corrupt file", + nullptr, ARGTYPE_BOOL, ARG_NOMINMAX, nullptr + }, }; const std::vector > kCoursePointTypeMapping = { diff --git a/reference/format3.txt b/reference/format3.txt index 79d05c79f..c3316d0f3 100644 --- a/reference/format3.txt +++ b/reference/format3.txt @@ -274,6 +274,8 @@ file -wrw-- garmin_fit fit Flexible and Interoperable Data Transfer (FIT) Activi https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html option garmin_fit allpoints Read all points even if latitude or longitude is missing boolean https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html#fmt_garmin_fit_o_allpoints +option garmin_fit recoverymode Attempt to recovery data from corrupt file boolean https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin_fit.html#fmt_garmin_fit_o_recoverymode + file rw---- flysight csv FlySight GPS File xcsv https://www.gpsbabel.org/WEB_DOC_DIR/fmt_flysight.html option flysight snlen Max synthesized shortname length integer 1 https://www.gpsbabel.org/WEB_DOC_DIR/fmt_flysight.html#fmt_flysight_o_snlen diff --git a/reference/help.txt b/reference/help.txt index 09ca5dbbf..91962ad3a 100644 --- a/reference/help.txt +++ b/reference/help.txt @@ -145,6 +145,7 @@ File Types (-i and -o options): timeadj (integer sec or 'auto') Barograph to GPS time diff garmin_fit Flexible and Interoperable Data Transfer (FIT) Act allpoints (0/1) Read all points even if latitude or longitude is m + recoverymode (0/1) Attempt to recovery data from corrupt file flysight FlySight GPS File snlen Max synthesized shortname length snwhite (0/1) Allow whitespace synth. shortnames diff --git a/testo.d/garmin_fit.test b/testo.d/garmin_fit.test index 3024edd4a..876317ca1 100644 --- a/testo.d/garmin_fit.test +++ b/testo.d/garmin_fit.test @@ -26,7 +26,7 @@ compare ${REFERENCE}/track/wahoo-element-bolt.gpx ${TMPDIR}/fit-sample-wahoo-ele gpsbabel -i garmin_fit -f ${REFERENCE}/track/garmin-oregon-700.fit -o gpx -F ${TMPDIR}/fit-sample-garmin-oregon-700.gpx compare ${REFERENCE}/track/garmin-oregon-700-output.gpx ${TMPDIR}/fit-sample-garmin-oregon-700.gpx -gpsbabel -i garmin_fit -f ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.fit -o gpx -F ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx +gpsbabel -i garmin_fit,recoverymode -f ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.fit -o gpx -F ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx 2>/dev/null compare ${REFERENCE}/track/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx ${TMPDIR}/lezyne_super_gps-garmin_fit-sample-bad-endian.gpx # diff --git a/xmldoc/formats/options/garmin_fit-recoverymode.xml b/xmldoc/formats/options/garmin_fit-recoverymode.xml new file mode 100644 index 000000000..18c96cc28 --- /dev/null +++ b/xmldoc/formats/options/garmin_fit-recoverymode.xml @@ -0,0 +1,13 @@ + +In the default mode the reader will issue a fatal error if it encounters indications of a corrupt file. These indications include: + +a bad Header or File CRC +a bad endian field +an attempt to use a message type that hasn't been previously defined +an attempt to read when the data section doesn't have sufficient data +an attempt to read past the end of file + +In recovery mode if we encounter a CRC error we will ignore it. If we encounter one of the other errors we will abort +read processing and continue. This allows any writer to use data that +was recovered previous to the read abort. + -- 2.30.2